Skip to content

restrict AIX use of clang to 26+ instead of 25+#4307

Merged
sxa merged 2 commits intonodejs:mainfrom
sxa:clangaix_v26only
Apr 22, 2026
Merged

restrict AIX use of clang to 26+ instead of 25+#4307
sxa merged 2 commits intonodejs:mainfrom
sxa:clangaix_v26only

Conversation

@sxa
Copy link
Copy Markdown
Member

@sxa sxa commented Apr 22, 2026

Draft until tested. Building node 25 with clang results in a build failure after #4286 selected clang for 25+ (like the other platforms) instead of 26+

clang++: error: unknown argument '-mno-popcntb'; did you mean '-mno-popcntd'?
clang++: error: unknown argument: '-fno-extern-tls-init'

Other option would be to have a separate "if 26 and AIX" section outside the 25+ block but I think this is simpler.

Signed-off-by: Stewart X Addison <sxa@ibm.com>
@sxa sxa marked this pull request as draft April 22, 2026 13:13
@sxa sxa self-assigned this Apr 22, 2026
@sxa sxa requested review from abmusse and richardlau April 22, 2026 13:20
Comment thread jenkins/scripts/select-compiler.sh Outdated
export CXX="clang++"
echo "Compiler set to Clang" `${CXX} -dumpversion`
fi
return
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may need the return to be inside the if block for the fall through to work as expected.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Signed-off-by: Stewart X Addison <sxa@ibm.com>
@sxa sxa marked this pull request as ready for review April 22, 2026 13:35
@sxa sxa merged commit 7ef34ea into nodejs:main Apr 22, 2026
2 checks passed
@abmusse
Copy link
Copy Markdown
Contributor

abmusse commented Apr 22, 2026

Thanks for fixing this up @sxa!

@sxa
Copy link
Copy Markdown
Member Author

sxa commented Apr 22, 2026

https://ci.nodejs.org/job/node-stress-single-test/nodes=aix72-power9/687/console is runnign through ok with 25 so I think this is ok - will also keep an eye on the 26 runs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants